doc: Clarify EOF condition for AsyncReadExt::read_buf #3850
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Darksonn
reviewed
Jun 9, 2021
tokio/src/io/util/async_read_ext.rs
Outdated
Comment on lines
183
to
186
| /// On a successful read, the number of read bytes is returned. If the | ||
| /// supplied buffer is not empty and the function returns `Ok(0)` then | ||
| /// the source has reached an "end-of-file" event. | ||
| /// remaining capacity of the supplied buffer is greater than 0 and | ||
| /// the function returns `Ok(0)` then the source has reached an | ||
| /// "end-of-file" event. |
There was a problem hiding this comment.
I think it would read even better if we made a list with the two cases like we do on AsyncReadExt::read.
There was a problem hiding this comment.
👍 I pulled the comment down from read, but changed the second point for read_buf because I think the concept of "remaining capacity" makes more sense for BufMut than length since a BufMut has the concept of initialized length and uninitialized remaining capacity.
Here's the change for that line:
- 2. The buffer specified was 0 bytes in length.
+ 2. The buffer specified had a remaining capacity of zero.
Darksonn
added
A-tokio
Squash into previous commit
Darksonn
reviewed
Jun 9, 2021
tokio/src/io/util/async_read_ext.rs
Outdated
| /// supplied buffer is not empty and the function returns `Ok(0)` then | ||
| /// the source has reached an "end-of-file" event. | ||
| /// If the return value of this method is `Ok(n)`, then it must be | ||
| /// guaranteed that `0 <= n <= buf.len()`. A nonzero `n` value indicates |
There was a problem hiding this comment.
The BufMut trait has no len method.
There was a problem hiding this comment.
Ah, you're right. I'll update.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Motivation
The current documentation for
read_bufsays:This confused me since I took "empty" to mean that the buffer had no readable data in it (like a vec with len() == 0).
I think part of the confusion is due to buffers like
bytes::BytesMutusing "empty" to refer to buffers that haveno readable data (i.e. a newly created buffer that hasn't been written to yet).
After reading the source code and chatting with @Darksonn it's clear that
Ok(0)is returned when there's no moreroom in the buffer to write additional bytes. This is "empty" in the sense that an empty in the sense that an empty struct is,
but not in the sense that an empty vec is (which IMO is a closer mental model to a buffer).
Solution
I propose we change the above sentence to read:
The use of "remaining capacity" here is consistent with the phrasing
bytes::BytesMutuses.